Skip to content

Generate debug port in range for Chrome DevTools websocket - iOS #3275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 5, 2018

Conversation

petekanev
Copy link
Contributor

@petekanev petekanev commented Dec 18, 2017

When generating a port to start a websocket on, use the same logic as is present in the android debug service - get the first available available port in a range, and reuse it for the next debug session of the same application if it is still available

Fixes #3183 and #3261

@petekanev petekanev force-pushed the pete/generate-debug-port-ios branch from a163224 to 09e0025 Compare December 18, 2017 14:06
@petekanev petekanev changed the base branch from master to release December 18, 2017 14:07
@petekanev petekanev force-pushed the pete/generate-debug-port-ios branch from 09e0025 to b37d93b Compare December 18, 2017 14:11
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After handling comment.
Don't forget to create a PR to master branch as well.

@@ -48,6 +49,12 @@ const createTestInjector = (): IInjector => {
on: (event: string | symbol, listener: Function): any => undefined
});

testInjector.register("net", {
getAvailablePortInRange: function (startPort: number, endPort?: number): any {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should return Promise here.
And also you can use lambda syntax here:

testInjector.register("net", {
    getAvailablePortInRange: async (startPort: number, endPort?: number): Promise<number> => 41000
}

@@ -212,7 +213,7 @@ export class IOSDebugService extends DebugServiceBase implements IPlatformDebugS
this.$logger.info("'--chrome' is the default behavior. Use --inspector to debug iOS applications using the Safari Web Inspector.");
}

this._socketProxy = await this.$socketProxyFactory.createWebSocketProxy(this.getSocketFactory(device));
this._socketProxy = await this.$net.getAvailablePortInRange(41000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bypasses a lot of logic, please check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, must have pushed the wrong thing

Copy link
Contributor

@KristianDD KristianDD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at comment.

@petekanev petekanev changed the title Generate debug port in range for Chrome DevTools websocket - iOS [Don't merge] Generate debug port in range for Chrome DevTools websocket - iOS Dec 18, 2017
Copy link
Contributor

@KristianDD KristianDD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After comments

@petekanev petekanev force-pushed the pete/generate-debug-port-ios branch from b37d93b to fe1d39d Compare December 18, 2017 15:52
@petekanev petekanev changed the title [Don't merge] Generate debug port in range for Chrome DevTools websocket - iOS Generate debug port in range for Chrome DevTools websocket - iOS Dec 18, 2017
petekanev and others added 2 commits January 4, 2018 14:31
…evtools

When generating a port to start a websocket on, use the same logic as is present in the android debug service - get the first available available port in a range, and reuse it for the next debug session of the same application if it is still available
…addapter

Check for available port is done on this adapter and by default server is launched on "::" which is IPv6's any adapter
@mbektchiev mbektchiev force-pushed the pete/generate-debug-port-ios branch from 08eb7f3 to a82b4f1 Compare January 4, 2018 12:31
Copy link
Contributor

@mbektchiev mbektchiev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it along with #3281 because they are needed for the latest debugging improvements in ios-runtime

Copy link
Contributor

@Mitko-Kerezov Mitko-Kerezov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay to me

@mbektchiev mbektchiev merged commit 96f476c into release Jan 5, 2018
@mbektchiev mbektchiev deleted the pete/generate-debug-port-ios branch January 5, 2018 07:09
mbektchiev pushed a commit that referenced this pull request Jan 5, 2018
* fix generating port for debug socket when debugging ios with chrome devtools

When generating a port to start a websocket on, use the same logic as is present in the android debug service - get the first available available port in a range, and reuse it for the next debug session of the same application if it is still available

* fix(device-socket): iOS Socket proxy should listen on IPv4 localhost adapter

Check for available port is done on this adapter and by default server is launched on "::" which is IPv6's any adapter
dtopuzov pushed a commit that referenced this pull request Jan 10, 2018
* Generate debug port in range for Chrome DevTools websocket - iOS (#3275)

* fix generating port for debug socket when debugging ios with chrome devtools

When generating a port to start a websocket on, use the same logic as is present in the android debug service - get the first available available port in a range, and reuse it for the next debug session of the same application if it is still available

* fix(device-socket): iOS Socket proxy should listen on IPv4 localhost adapter

Check for available port is done on this adapter and by default server is launched on "::" which is IPv6's any adapter

* Fix debug device/web socket closing (#2897)

When we close the websocket we have to destroy the device socket as well.
We need to wait the close event on the device socket instead of end.

* fix(ios-inspector): Correctly update cached inspector package to latest compatible version (#3302)

* Currently it is never updated and is left at the first version ever installed in `~/.local`.
As a result newly published inspector packages are not used unless the user installs
them explicitly in the project or deletes the cache manually.
* The iOS inspector package version must be determined according to project's iOS runtime
version instead of the version of CLI as it has been till now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants